-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make copy/paste work for source code #12191
Conversation
Fix regression casued by go-gitea#12047 so copy/paste works properly in all browsers. Fixes go-gitea#12184 Also while looking at this I saw a small display issue for blame view. I think go-gitea#12023 was merged into original PR through an update branch before go-gitea#12047 was merged and made one of the css ruules not apply anymore.
Looking pretty good now. One thing I've noticed it that copying two or more lines out of a diff appends a trailing newline in both Firefox and Chrome. It does not do that on the file view, and I think that newline should ideally not be copied. |
@silverwind will take a look |
Actually can't reproduce the diff issue -- can you link to an example to copy? |
Any diff like https://try.gitea.io/silverwind/updates/commit/7c5b7c608d95fa85644c05d9a33355511a3ae492 Select at least two lines, copy them and paste them into an editor and you'll notice the cursor being on the line after the two lines. |
Can't reproduce on this PR @silverwind @mrsdizzie Copying from PR diff view where you have [+] buttons for code comment still copies the '+' character from it on every line. This causes Firefox to place it in separate line while for Chromium it's placed on same line, before actual content. Chromium:
Firefox:
Desired output:
|
@CirnoT ah OK -- appears that it has always been like this for PR diff rather than a regression from recent changes. I'll try and fix it for this PR too since it should just be similar to regular diff |
Generally, adding |
…ot visually selected
@CirnoT should be fixed for PR diff -- seems ok in Firefox/Chrome from testing. |
Regarding the newline copying issue in diff, it seems to be because each line in the diff has a trailing newline which is not present on regular view lines. In regular file, there's no newline after the last chroma token (last <code> <span class="nt">"license"</span><span class="p">:</span> <span class="s2">"BSD-2-Clause"</span><span class="p">,</span></code> but in diff view, there's a newline after it <span class="mono wrap"> <span class="s2">"license"</span><span class="err">:</span> <span class="s2">"BSD-2-Clause"</span><span class="err">,</span>
</span> Can we chop it off maybe? |
@silverwind I pushed a change to fix that if you can test -- I see the new lines in the source but don't experience the copy/paste issue so can't verify that it would fix it. |
@mrsdizzie It works fine now, thanks. |
Actually the newest commit broke newlines again, they're no longer present on copy. |
Trailing newline is now fixed for me. @CirnoT which newlines exactly? |
Between lines obviously |
ah ok I see sheesh sorry, will fix next commit |
I mean diff or code view, or both? It does seem to work fine here in Firefox 79 and Chrome 83. |
|
Copying it outside browser however produces this:
|
I don't understand. Where do you paste to get these vertical lines? |
Just here in GH textarea, I guess it adds it for some reason? Pasting it in raw <textarea> on simple HTML page results in
Note the missing empty newline |
Indeed, I see very strange paste results when copy/pasting inside Firefox but it does work normally when I paste into an external editor, wtf. |
What OS if I can ask @silverwind. Neither me nor @mrsdizzie can't reproduce your original report about stray newline at the end? |
Final newline issue was both in Windows and macOS, Firefox and Chrome. Thought in Chrome, not always. |
Must be some GH feature; try it on any other site that has simple unmodified <textarea> element. |
Windows here, could not reproduce on Chrome and Firefox alike. |
I can reproduce it If i try, by which I mean specifically try to move cursor so that it catches empty element on next line by ending drag selection on line below. I wouldn't consider that a bug however? |
Yeah apparantly GH textarea does magic to break pasted text, nice. Try pasting into a pristine textarea like https://jsfiddle.net/silverwind/znpcLo4w/. Not sure what else to tell you, I start selection on bottom right of the line, move cursor to top left, hit copy and then paste. Current version is LGTM. |
No, the current version does not preserve empty newlines on diff view |
@CirnoT I believe that last push should fix copying newline in diff again |
Ah right, I can see the empty newline issue too. Testing latest fix. |
Confirming everything working now for me. |
Works fine now here too |
Make lg-tm work |
Fix regression casued by #12047 so copy/paste works properly in all browsers.
Fixes #12184
Also while looking at this I saw a small display issue for blame view. I think #12023 was merged into original PR through an update branch before #12047 was merged and made one of the css rules not apply anymore.
Also fix existing and unrelated bug where copy/paste from PR diff would select the + symbol from add comment features